-
Notifications
You must be signed in to change notification settings - Fork 427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce DynamicMapAttribute #868
Conversation
object.__setattr__(self, name, value) if name in self.get_attributes() else super().__setattr__(name, value) | ||
|
||
def serialize(self, values): | ||
if not isinstance(values, type(self)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this blob transforms a raw dict into a map attr object right? is this not shared with raw map attrs?
pynamodb/attributes.py
Outdated
instance.attribute_values = {} # clear any defaults | ||
instance._set_attributes(**values) | ||
values = instance | ||
rval = AttributeContainer.serialize(values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this serializes every defined attribute
pynamodb/attributes.py
Outdated
instance._set_attributes(**values) | ||
values = instance | ||
rval = AttributeContainer.serialize(values) | ||
for attr_name in values: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this loop serializes the left over, undefined attrs
pynamodb/attributes.py
Outdated
values = instance | ||
rval = AttributeContainer.serialize(values) | ||
for attr_name in values: | ||
if attr_name not in self.get_attributes(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we dedup this with MapAttribute.serialize
?
We might also want to add a mention here: https://pynamodb.readthedocs.io/en/latest/attributes.html#map-attributes |
add discriminator + dynamicmapattr test |
120af41
to
654d854
Compare
docs/attributes.rst
Outdated
|
||
car = CarInfo(make='Make-A', model='Model-A', year=1975) | ||
other_car = CarInfo(make='Make-A', model='Model-A', year=1975, seats=3) | ||
|
||
`As with a model and its top-level attributes <https://github.com/pynamodb/PynamoDB/blob/master/docs/quickstart.rst#changing-items>`_, a PynamoDB MapAttribute will ignore sub-attributes it does not know about during deserialization. As a result, if the item in DynamoDB contains sub-attributes not declared as properties of the corresponding MapAttribute, save() will cause those sub-attributes to be deleted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This applies only to MapAttribute
subclasses, right? Might want to reorder the DynamicMapAttribute
content underneath so as not to confuse
class TestDynamicMapAttribute: | ||
|
||
class CreatedAtTestModel(Model): | ||
class CreatedAtMap(DynamicMapAttribute): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also test with a subclass of CreatedAtMap
to test one more level of inheritance?
# Continue to serialize NULL values in "raw" map attributes for backwards compatibility. | ||
# This special case behavior for "raw" attributes should be removed in the future. | ||
for attr_name in values: | ||
if attr_name not in self.get_attributes(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is a new branch that should always return True
for MapAttribute
and sometimes return True
for DynamicMapAttribute
. Calling it out because there's some perf impact, though I think it should be marginal
DynamicMapAttribute is a map attribute that supports declaring attributes (like an AttributeContainer),
but will also store any other values that are set on it (like a raw MapAttribute).
This class lets you create a "partially typed" map attribute so that it can store items that are not just primitive python types.